pyproject.toml (1)

69-69: Align mypy target python with runtime.

requires-python is ">=3.11" but mypy is set to "3.10". Set to 3.11 to avoid mismatches in type inference.

Suggested:

[tool.mypy]
python_version = "3.11"

connector_builder_agents/src/run.py (4)

28-31: Use a collision‑free session id.

int(time.time()) can collide under concurrency. Prefer UUID.

-def generate_session_id() -> str:
-    """Generate a unique session ID based on current timestamp."""
-    return f"unified-mcp-session-{int(time.time())}"
+import uuid
+
+def generate_session_id() -> str:
+    """Generate a unique session ID."""
+    return f"unified-mcp-session-{uuid.uuid4()}"

---

98-105: Avoid CWD‑relative prompt path; use the module constant.

This will fail if run outside repo root. Reuse ROOT_PROMPT_FILE_STR.

-        prompt_file = Path("./prompts") / "root-prompt.md"
-        prompt = prompt_file.read_text(encoding="utf-8") + "\n\n"
-        prompt += instructions
+        from .constants import ROOT_PROMPT_FILE_STR
+        prompt = ROOT_PROMPT_FILE_STR + "\n\n" + instructions

---

200-234: Add safety limits and trim message history to prevent unbounded growth.

• Loop can run indefinitely if agents never mark completion.
• message_history grows without bounds.

-        while not is_complete(session_state):
+        from .constants import MAX_CONNECTOR_BUILD_STEPS
+        MAX_HISTORY = 200  # local cap; consider promoting to constants
+        while not is_complete(session_state) and iteration_count < MAX_CONNECTOR_BUILD_STEPS:
@@
-            session_state.message_history.extend(run_result.new_messages())
+            session_state.message_history.extend(run_result.new_messages())
+            if len(session_state.message_history) > MAX_HISTORY:
+                session_state.message_history = session_state.message_history[-MAX_HISTORY:]
@@
-        return all_run_results
+        if not is_complete(session_state):
+            update_progress_log("⏹ Reached max iterations; stopping.", session_state)
+        return all_run_results

---

236-241: Avoid sys.exit in run_manager_developer_build.

Return cleanly so callers control process termination.

-    except KeyboardInterrupt:
+    except KeyboardInterrupt:
         update_progress_log("\n🛑 Build terminated (ctrl+c input received).", session_state)
-        sys.exit(0)
+        return all_run_results if 'all_run_results' in locals() else []

connector_builder_agents/src/agents.py (2)

106-121: Handle errors from delegated developer runs.

If the developer toolchain fails (tool error/timeout), log via log_tool_failure and propagate a concise message to the manager to decide next steps.

-        result = await developer_agent.run(
+        try:
+            result = await developer_agent.run(
                 assignment_description,
                 message_history=ctx.deps.message_history,
                 deps=ctx.deps,
             )
+        except Exception as e:
+            update_progress_log(
+                f"🛠️ Delegation failed for '{assignment_title}': {e}", ctx.deps
+            )
+            return f"Delegation failed: {e}"

---

118-120: Cap shared message history to avoid ballooning.

Same concern as in run.py; trim after extending.

-        ctx.deps.message_history.extend(result.new_messages())
+        ctx.deps.message_history.extend(result.new_messages())
+        if len(ctx.deps.message_history) > 200:
+            ctx.deps.message_history = ctx.deps.message_history[-200:]

connector_builder_agents/src/tools.py (2)

22-31: Avoid mutable default for message_history.

Use Field(default_factory=list) and drop manual kwargs handling.

-    message_history: list = []
+    message_history: list = Field(default_factory=list)
@@
-        message_history = kwargs.get("message_history", [])
@@
         super().__init__(
             workspace_dir=workspace_dir,
             execution_log_file=execution_log_file,
-            message_history=message_history,
+            # Let Pydantic default_factory create a fresh list per instance
             start_time=start_time,
             **kwargs,
         )

Also applies to: 40-44

---

58-73: Duplicate connector‑builder MCP servers; reuse a single instance.

Two identical MCPServerStdio("uv", ["run","airbyte-connector-builder-mcp"]) are spawned, contradicting the “reuse instances” docstring and wasting resources.

-connector_builder_dev = MCP_CONNECTOR_BUILDER_FOR_DEVELOPER()
-connector_builder_manager = MCP_CONNECTOR_BUILDER_FOR_MANAGER()
+connector_builder = MCP_CONNECTOR_BUILDER_FOR_DEVELOPER()
@@
-    all_servers = [
+    all_servers = [
         # MCP_PLAYWRIGHT_WEB_BROWSER(),
-        connector_builder_dev,
-        connector_builder_manager,
+        connector_builder,
         filesystem_server,
     ]
@@
-    manager_servers = [
-        connector_builder_manager,
+    manager_servers = [
+        connector_builder,
         filesystem_server,
     ]
@@
-    developer_servers = [
+    developer_servers = [
         # MCP_PLAYWRIGHT_WEB_BROWSER(),
-        connector_builder_dev,
+        connector_builder,
         filesystem_server,
     ]

Or drop the separate lambdas and define a single MCP_CONNECTOR_BUILDER factory.

Also applies to: 106-127

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

connector_builder_agents/src/agents.py (1)

120-120: Consider type-safe result data handling.

The conversion str(result.data) assumes result.data is present and meaningful. If result.data is None or an unexpected type, the conversion succeeds silently but may return unhelpful values like "None".

For better type safety, consider explicit handling:

-        return str(result.data)
+        if result.data is None:
+            return "Task completed (no data returned)"
+        return str(result.data)

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

connector_builder_agents/src/agents.py (2)

106-110: Add error handling for developer agent invocation.

The developer_agent.run() call lacks error handling. If the developer agent encounters an exception, it will propagate uncaught, potentially causing the manager to fail without proper logging or recovery.

The previous review comment on this issue remains valid and unresolved. Consider wrapping the invocation in a try-except block as suggested in the earlier review.

---

118-118: Limit message history growth.

The ctx.deps.message_history.extend(result.new_messages()) call allows unbounded growth. In long-running sessions with many delegations, this could lead to excessive memory consumption and performance degradation.

The previous review comment on this issue remains valid and unresolved. Consider implementing a mechanism to prune old messages or set a maximum history size as suggested in the earlier review.

connector_builder_agents/src/agents.py (1)

122-150: Consider enhancing the return value.

The tool returns a static string "Status reported to manager" regardless of the status content. While functional, a more informative return value could include a summary of the reported status.

For example:

-        return "Status reported to manager"
+        return f"Reported to manager: {short_status} (full_success={is_full_success}, blocked={is_blocked})"

This would provide better context in the message history and agent logs.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

connector_builder_agents/src/run.py (1)

145-145: Previous critical issue not addressed: blocking I/O and process termination.

The blocking input() call at Line 145 and sys.exit(0) at Line 154 were flagged in the previous review but remain unresolved.

• Line 145: input() blocks the async event loop, preventing concurrent operations.
• Line 154: sys.exit(0) terminates the entire process ungracefully, preventing cleanup and making the function non-reusable.

Apply this diff to address both issues:

+import asyncio
+
 async def run_interactive_build(
     prompt: str,
     model: str,
     session_id: str,
 ) -> None:
     """Run the agent using interactive mode with conversation loop."""
     workspace_dir = get_workspace_dir(session_id)
     session_state = create_session_state(workspace_dir)
 
     all_mcp_servers, _, _ = create_session_mcp_servers(session_state)
     agent = Agent(
         model,
         name="MCP Connector Builder",
         deps_type=SessionState,
         system_prompt=(
             "You are a helpful assistant with access to MCP tools for building Airbyte connectors."
         ),
     )
 
     for mcp_server in all_mcp_servers:
         agent.toolsets.append(mcp_server)
 
     input_prompt: str = prompt
     while True:
         update_progress_log("\n⚙️  AI Agent is working...", session_state)
         try:
             result = await agent.run(
                 input_prompt,
                 message_history=session_state.message_history,
                 deps=session_state,
             )
 
             session_state.message_history.extend(result.new_messages())
 
             update_progress_log(f"\n🤖  AI Agent: {result.output}", session_state)
 
-            input_prompt = input("\n👤  You: ")
+            loop = asyncio.get_event_loop()
+            input_prompt = await loop.run_in_executor(None, input, "\n👤  You: ")
             if input_prompt.lower() in {"exit", "quit"}:
                 update_progress_log("☑️ Ending conversation...", session_state)
                 break
 
         except KeyboardInterrupt:
             update_progress_log(
                 "\n🛑 Conversation terminated (ctrl+c input received).", session_state
             )
-            sys.exit(0)
+            return None
 
     return None

Also applies to: 150-154

connector_builder_agents/src/run.py (1)

50-50: Consider preserving type information in return signatures.

The return types have been loosened from list[RunResult] to list (Line 165) and from list[RunResult] | None to list | None (Line 50). This removes valuable type information and reduces type safety.

If RunResult is no longer available due to the framework migration, consider defining a replacement type or using a more specific type hint.

If pydantic-ai provides a result type, use it:

+from pydantic_ai import RunResult  # or equivalent
+
-) -> list | None:
+) -> list[RunResult] | None:

-) -> list:
+) -> list[RunResult]:

Also applies to: 165-165

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

connector_builder_agents/src/evals/task.py (1)

54-54: Rename dictionary key to match attribute. The code now uses final_result.output but the result dict still uses "final_output"—consider renaming it to "output" for consistency.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro